We can replace this will Buf, as used by other HashiCorp products.

No objection for this, if the tool works similarly we'll be good to go with it

We should add a constant for "v2" so that we can comment on what this value means.

Sounds feasible to me yeah, we can also use that constant from Packer core, so that seems good to me

Any particular reason why you chose the flag parsing logic above as opposed to adding a flag.Parse(args) when the issued sub command is "start".

Then inside of the Start function when you construct the server you can specify that it should use protobuf for serialization. You will need to add a new filed to the server to track this. It can be a bool if you go the UseProtoBuf name or a string if you go ProtcolVersion. This way in the common rpc code the server can check if the field is set instead of the global rpc.UseProto variable.

So flag I thought of using it yes, but in the end I went with this approach as the arguments are passed as a parameter to this function, and flag.Parse works on the raw arguments from os.Args, besides I think the order had some impact on what was returned (i.e. not sure you can have options anywhere in the command-line), so I thought might as well do something ad-hoc that'd let us place the option wherever.
Besides that's not something that will be used outside Packer, and that'll possibly disappear someday (when Gob is out of the picture), so I thought it was maybe easier to do it this way rather than change how we manage flags in there today.

That being said there are merits to that option of refactoring to handle flags properly, just thought that today was not the day for that, but it should be feasible. Let me know if you think this'd be valuable and I can probably spend some time on that option.

Ah I always confuse these args given the whole Run, RunCommand methods our CLI uses. Thanks for the details on why you wrote it this way. I don't think it is important at this very moment given that there is only one flag. But you wrote a decent snippet of code so I was curious why. Sorry for not looking at Run() too much.

When ready I guess we can add the flag.Parse to the Run method and just create a variable for the UseProto flag.

I believe we should store this information on the server and client as fields. As opposed to using this global variable.
In Packer when we check if the plugins support protocol_version "v2" we can config the client using PluginClientConfig to UseProto. The server as I mentioned above when passed the --protobuf flag will create the server with the UseProto field set to true.

The code for the client and server in common then start to look like

 func (p *commonClient) ConfigSpec() hcldec.ObjectSpec {
	// Legacy: this will need to be removed when we discontinue gob-encoding
	//
	// This is required for backwards compatibility for now, but using
	// gob to encode the spec objects will fail against the upstream cty
	// library, since they removed support for it.
	//
	// This will be a breaking change, as older plugins won't be able to
	// communicate with Packer any longer.
	if !c.UseProto {  // or if c.ProtocolVersion != "v2"
		log.Printf("[DEBUG] - common: receiving ConfigSpec as gob")
		res := hcldec.ObjectSpec{}
		err := gob.NewDecoder(bytes.NewReader(resp.ConfigSpec)).Decode(&res)
		if err != nil {
			panic(fmt.Errorf("failed to decode HCL spec from gob: %s", err))
		}
		return res
	}
}

....

func (s *commonServer) ConfigSpec(_ interface{}, reply *ConfigSpecResponse) error {
// Legacy: this will need to be removed when we discontinue gob-encoding
	//
	// This is required for backwards compatibility for now, but using
	// gob to encode the spec objects will fail against the upstream cty
	// library, since they removed support for it.
	//
	// This will be a breaking change, as older plugins won't be able to
	// communicate with Packer any longer.
	if !s.UseProto {  // or if s.ProtocolVersion != "v2"
		log.Printf("[DEBUG] - common: receiving ConfigSpec as gob")
		res := hcldec.ObjectSpec{}
		err := gob.NewDecoder(bytes.NewReader(resp.ConfigSpec)).Decode(&res)
		if err != nil {
			panic(fmt.Errorf("failed to decode HCL spec from gob: %s", err))
		}
		return res
	}
}

I have to dive deeper into the datasources but I believe it embeds a Common client and a Common server if so I believe the fields may be accessible on the datasource.

Everything embeds commonClient/commonServer, the datasource has some ad-hoc usage though as the config specs for the returned types are handled separately, while ConfigSpec is for the parameters exchanged with the plugin.

I'm not sure as to the proposed change though, both clients and servers need to be in line with the protocol used, the current approach means UseProto can be global, it however needs to be set to either true or false ASAP in the lifecycle of the processes, that means at command start for plugins, and once the discovery is done for Packer.

We can explore this path, but I'm not sure we need to make this an option of the structures here, we'd still need to know what value to give that arg when we create the structure.

E.g. from a plugin perspective, I'm pretty certain we won't have access to the command-flags at the point where we create the RPC structs, so we'll probably need to either forward it, or keep it globally. If we keep it global, are there merits to this approach, as we'll copy a global flag into a structure?

That being said, I can still take a look at it, maybe there's a clean way to make it work.

I'm not sure as to the proposed change though, both clients and servers need to be in line with the protocol used, the current approach means UseProto can be global, it however needs to be set to either true or false ASAP in the lifecycle of the processes, that means at command start for plugins, and once the discovery is done for Packer.

This is correct. The caller would be responsible for initializing the server and client with the protocol enabled so since Packer is the control here. When we identify that all plugins support protocol v2 we setup Packer to start create a client for the plugin with the protocol set to use protobuf. The execute command that starts the plugin with the --protobuf flag. This will tell the SDK that the server should use protobuf. At this point the client and server will be in line.

E.g. from a plugin perspective, I'm pretty certain we won't have access to the command-flags at the point where we create the RPC structs, so we'll probably need to either forward it, or keep it globally. If we keep it global, are there merits to this approach, as we'll copy a global flag into a structure?